Skip to content

HDDS-7807. TarContainerPacker closes streams multiple times #4193

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
adoroszlai merged 2 commits into from
Jan 20, 2023
Merged

HDDS-7807. TarContainerPacker closes streams multiple times #4193

adoroszlai merged 2 commits into from
Jan 20, 2023

Conversation

@adoroszlai
Copy link
Contributor

@adoroszlai adoroszlai commented Jan 19, 2023

What changes were proposed in this pull request?

TarContainerPacker closes the underlying GrpcOutputStream multiple times:

datanode_1  | java.lang.IllegalStateException: call already closed
datanode_1  | 	at org.apache.ratis.thirdparty.com.google.common.base.Preconditions.checkState(Preconditions.java:502)
datanode_1  | 	at org.apache.ratis.thirdparty.io.grpc.internal.ServerCallImpl.closeInternal(ServerCallImpl.java:218)
datanode_1  | 	at org.apache.ratis.thirdparty.io.grpc.internal.ServerCallImpl.close(ServerCallImpl.java:211)
datanode_1  | 	at org.apache.ratis.thirdparty.io.grpc.stub.ServerCalls$ServerCallStreamObserverImpl.onCompleted(ServerCalls.java:395)
datanode_1  | 	at org.apache.hadoop.ozone.container.replication.GrpcOutputStream.close(GrpcOutputStream.java:106)
datanode_1  | 	at org.apache.hadoop.ozone.container.keyvalue.TarContainerPacker.pack(TarContainerPacker.java:180)
datanode_1  | 	at org.apache.hadoop.ozone.container.keyvalue.KeyValueContainer.packContainerToDestination(KeyValueContainer.java:902)
datanode_1  | 	at org.apache.hadoop.ozone.container.keyvalue.KeyValueContainer.exportContainerData(KeyValueContainer.java:630)
datanode_1  | 	at org.apache.hadoop.ozone.container.keyvalue.KeyValueHandler.exportContainer(KeyValueHandler.java:1038)
datanode_1  | 	at org.apache.hadoop.ozone.container.ozoneimpl.ContainerController.exportContainer(ContainerController.java:167)
datanode_1  | 	at org.apache.hadoop.ozone.container.replication.OnDemandContainerReplicationSource.copyData(OnDemandContainerReplicationSource.java:75)
datanode_1  | 	at org.apache.hadoop.ozone.container.replication.GrpcReplicationService.download(GrpcReplicationService.java:60)

The exception seems harmless, but it may be confusing and pollutes the log.

This can be reproduced by adding assertion about number of close() calls in TestTarContainerPacker:

AssertionFailedError: expected: <1> but was: <2>
	at org.junit.jupiter.api.AssertionUtils.fail(AssertionUtils.java:55)
	at org.junit.jupiter.api.AssertionUtils.failNotEqual(AssertionUtils.java:62)
	at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:150)
	at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:145)
	at org.junit.jupiter.api.Assertions.assertEquals(Assertions.java:527)
	at org.apache.ozone.test.SpyOutputStream.assertClosedExactlyOnce(SpyOutputStream.java:45)
	at org.apache.hadoop.ozone.container.keyvalue.TestTarContainerPacker.pack(TestTarContainerPacker.java:216)

This happens because TarArchiveInputStream and TarArchiveOutputStream both close the underlying in/output streams, so we only need to close these, not the underlying ones.

https://issues.apache.org/jira/browse/HDDS-7807

How was this patch tested?

Added assertions in unit test.

https://github.com/adoroszlai/hadoop-ozone/actions/runs/3960752311

@adoroszlai adoroszlai self-assigned this Jan 19, 2023
sodonnel
sodonnel approved these changes Jan 19, 2023
View reviewed changes
Copy link
Contributor

@sodonnel sodonnel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM if the CI is green.

@adoroszlai adoroszlai merged commit 099cd1d into apache:master Jan 20, 2023
@adoroszlai adoroszlai deleted the HDDS-7807 branch January 20, 2023 17:48
@adoroszlai
Copy link
Contributor Author

adoroszlai commented Jan 20, 2023

Thanks @sodonnel for the review.

errose28 added a commit to errose28/ozone that referenced this pull request Apr 20, 2023
@errose28
Merge branch 'master' into HDDS-7495
df2364f 
* master: (209 commits)
  HDDS-7097. Container scanner log output lacks useful information (apache#4169)
  HDDS-7813. Handle Mismatched Replicas (OPEN or CLOSING) of QUASI-CLOSED containers in RM (apache#4195)
  HDDS-7625. Do not compress OM/SCM checkpoints (apache#4130)
  HDDS-7801. Bucket not found when calling getKeyInfo with tenant context (apache#4189)
  HDDS-7807. TarContainerPacker closes streams multiple times (apache#4193)
  HDDS-7755. Ensure that acquired locks are always released. (apache#4191)
  HDDS-7804. UNHEALTHY replicas will not contribute to sufficient replication in RatisContainerReplicaCount (apache#4192)
  HDDS-7748. Rename OMFileRequest.addToOpenFileTable() to avoid misuse. (apache#4176)
  HDDS-7723. Refresh Keys and Certificate used in OzoneSecretManager after certificate renewed (apache#4179)
  HDDS-7788. Ratis OverReplicationHandler should exclude stale replicas (apache#4183)
  HDDS-7718. Bump Netty to 4.1.86 and gRPC to 1.51.1 (apache#4139)
  HDDS-7542. Refactor DefaultReplicationConfig (apache#4005)
  HDDS-7787. GetChecksum for EC files can fail intermittently with IndexOutOfBounds exception (apache#4180)
  HDDS-7754. Download of container is failing with SSL/TLS error during re-replication (apache#4174)
  HDDS-7455. ClassCastException: OzoneTokenIdentifier cannot be cast to String (apache#4159)
  HDDS-7441. Rename function names of retrieving metadata keys (apache#3918)
  HDDS-7722. FSO buckets fail to invalidate open file table cache when committing a key (apache#4156)
  HDDS-7774. Update outdated Trash documentation (apache#4172)
  HDDS-7761. EC: ReplicationManager - Use placementPolicy.replicasToRemoveToFixOverreplication in EC Over replication handler (apache#4166)
  HDDS-7775. EC: Exception encountered while deleting UNHEALTHY replica in Datanode (apache#4173)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sodonnel sodonnel sodonnel approved these changes

Assignees

@adoroszlai adoroszlai

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@adoroszlai @sodonnel